fix: validate the pod name in emit_script, not only the CLI#36
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make the public
emit_scriptvalidate its pod name, closing the deferred "harden the pod-cleanup trap" item. The pod name is embedded into shell in several places — most fragile the single-quotedEXITtrap (trap '... podman pod rm -f <pod> ... ; podman secret rm <pod>-<name> ...' EXIT) and the raw<pod>-<name>store names insecret create/secret rm. The CLI already validated it, butemit_script/EmitOptionsare public API, so a library caller could pass an unvalidated pod and produce a malformed/unsafe trap.Design:
planning/changes/2026-07-12.01-validate-pod-name-in-emit.md.Fix — validate the identifier, don't shell-escape it
A pod name is an identifier, not free text, so the honest-subset move is to reject a bad one (exactly how the codebase already treats secret/config names) rather than contort the trap to survive arbitrary metacharacters:
POD_NAME_PATTERN(^[A-Za-z0-9][A-Za-z0-9_.-]*$) moves fromcli.pytoemit.pyas the single source of truth;cli.pyimports it.emit_script's first statement validatesoptions.podand raisesUnsupportedComposeErroron a non-match, guarding every caller (library and CLI)..fullmatch(not.match), closing the trailing-newline hole ("p\n"matches a$-anchored.matchbut breaks the trap) — the same lesson as the secret-name injection fix.shlex.quoteis a no-op and the existing emission is provably safe.Testing
just test-ciat 100% coverage (354 tests),just lint-ciclean,just check-planningOK.emit_scriptnow raises on six adversarial pod values (single-quote injection, space,$(...), empty, leading dash, trailing newline); a valid pod still emits; the CLI's existing rejection (exit 2, same message) is unchanged.An adversarial whole-branch review confirmed the security property is fully closed — the guard is
emit_script's unconditional first statement, every char the pattern admits is inshlex.quote's safe class (so no nested quote ever enters the single-quoted trap), and both halves of every<pod>-<name>store name are validated.Docs / planning
Retires the trap-hardening entry from
deferred.md(its revisit trigger — a library-API test with adversarial pod values — is now met), and records the pod-name identifier constraint inarchitecture/supported-subset.md.